ci(shellcheck): add PR shell script linting#15
Merged
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro (Legacy) Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a ShellCheck GitHub Actions workflow and updates shell scripts to use bounded Main cohort
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
elibosley
force-pushed
the
codex/pr-shellcheck
branch
from
19e73c5 to
bcb2ee8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/menu_gui_common.sh (1)
295-296:⚠️ Potential issue | 🟠 MajorFix remaining SC2015 patterns to unblock ShellCheck.
Line 295, Line 296, Line 311, and Line 312 still use
A && B || C, which triggers the pipeline warnings and can mask command failure semantics.Proposed fix
- [ -w /proc/sys/kernel/sysrq ] && echo 1 > /proc/sys/kernel/sysrq || true - [ -w /proc/sysrq-trigger ] && echo o > /proc/sysrq-trigger || true + if [ -w /proc/sys/kernel/sysrq ]; then + echo 1 > /proc/sys/kernel/sysrq || true + fi + if [ -w /proc/sysrq-trigger ]; then + echo o > /proc/sysrq-trigger || true + fi @@ - [ -w /proc/sys/kernel/sysrq ] && echo 1 > /proc/sys/kernel/sysrq || true - [ -w /proc/sysrq-trigger ] && echo b > /proc/sysrq-trigger || true + if [ -w /proc/sys/kernel/sysrq ]; then + echo 1 > /proc/sys/kernel/sysrq || true + fi + if [ -w /proc/sysrq-trigger ]; then + echo b > /proc/sysrq-trigger || true + fiAlso applies to: 311-312
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/menu_gui_common.sh` around lines 295 - 296, Replace the problematic "A && B || true" pipeline patterns with explicit if statements to avoid masking failures (ShellCheck SC2015); e.g. change "[ -w /proc/sys/kernel/sysrq ] && echo 1 > /proc/sys/kernel/sysrq || true" and "[ -w /proc/sysrq-trigger ] && echo o > /proc/sysrq-trigger || true" (and the similar pair at lines 311–312) to "if [ -w /proc/sys/kernel/sysrq ]; then echo 1 > /proc/sys/kernel/sysrq; fi" and "if [ -w /proc/sysrq-trigger ]; then echo o > /proc/sysrq-trigger; fi" respectively so the write is attempted only when writable without using the && || pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@scripts/menu_gui_common.sh`:
- Around line 295-296: Replace the problematic "A && B || true" pipeline
patterns with explicit if statements to avoid masking failures (ShellCheck
SC2015); e.g. change "[ -w /proc/sys/kernel/sysrq ] && echo 1 >
/proc/sys/kernel/sysrq || true" and "[ -w /proc/sysrq-trigger ] && echo o >
/proc/sysrq-trigger || true" (and the similar pair at lines 311–312) to "if [ -w
/proc/sys/kernel/sysrq ]; then echo 1 > /proc/sys/kernel/sysrq; fi" and "if [ -w
/proc/sysrq-trigger ]; then echo o > /proc/sysrq-trigger; fi" respectively so
the write is attempted only when writable without using the && || pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro (Legacy)
Run ID: b19aa874-e7ce-412e-b116-3afdbd11a7a4
📒 Files selected for processing (5)
.github/workflows/shellcheck.ymlscripts/create_flash_boot.shscripts/create_internal_boot_user.shscripts/menu_gui_common.shscripts/zip.sh
- Purpose: ensure every pull request runs ShellCheck against tracked installer shell scripts. - Before: shell script linting was manual, and several existing informational ShellCheck findings would make a new gate fail immediately. - Problem: PRs could introduce shell issues without an automated signal, and adding the workflow without cleanup would create a permanently failing check. - Change: add a PR-level ShellCheck workflow using Ubuntu's shellcheck package and a SHA-pinned checkout action. - How: run ShellCheck over git-tracked scripts/*.sh files, fix existing ShellCheck findings in ZIP selection, menu dimension parsing, and zip metadata parsing comments.
elibosley
force-pushed
the
codex/pr-shellcheck
branch
from
bcb2ee8 to
c28f274
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ShellCheckworkflow that runs on every pull request and can be run manually.scripts/*.shfiles.actions/checkoutto the current v4 tag target SHA instead of adding another mutable action tag.Validation
bash -n scripts/create_flash_boot.sh scripts/create_internal_boot_user.sh scripts/menu_gui_common.sh scripts/zip.shshellcheck $(git ls-files 'scripts/*.sh')git diff --check HEAD~1..HEADSummary by CodeRabbit
Chores
Bug Fixes
Stability